nice!

🟡 Recommended — A violation whose traced sourceLocations is empty is silently discarded — it produces neither a diagnostic nor a dropped entry. anchorViolation returns traced.sourceLocations.map(...), so an empty list yields []; isFailure([]) is false, so the located branch runs with nothing to append and the dropped path is never taken. The doc comment above mapViolationsToDiagnostics still asserts such violations "are dropped (with a reason) rather than thrown," which is now inaccurate, and server.ts logs every dropped entry via log.warn, so a framework-only or unresolved violation now vanishes without any trace to the user.

Suggested change: Treat an empty sourceLocations as a drop with a reason — e.g. have anchorViolation return { reason: 'no source location (framework-only trace or unresolved source)' } when traced.sourceLocations.length === 0, so the existing dropped/log.warn path still surfaces it; or, if the silence is intentional, update the comment to match.

🟡 Recommended — retryOnLock calls AbortSignal.any([...]), which is only available from Node.js 18.17.0 (and 20.3.0), but this package declares engines.node as >= 18.0.0. On a Node 18.0–18.16 runtime the first lock-retry throws TypeError: AbortSignal.any is not a function, and retryOnLock sits on both the primary read path (readAssemblyDirectory) and the synth path, so the explorer would fail to read or synth an assembly there. The declared engine contract and the code's actual requirement disagree.

Suggested change: Confirm the intended minimum Node version. If Node 20 is the real target, raise engines.node accordingly via .projenrc.ts; if 18.0 must be supported, compose the deadline without AbortSignal.any (e.g. a manual setTimeout-driven AbortController plus an abort listener).

⚪ Optional — The SourceMapResolver constructor's JSDoc documents @param projectRoot - Absolute path to the project..., but the constructor's only parameter is index: AssemblyIndexWithSourceLocations. The projectRoot parameter no longer exists on this constructor, so the comment describes a signature that isn't there and misleads the next reader about what the resolver takes.

Suggested change: Replace the @param projectRoot block with one describing index; the attacker-influence rationale is still valuable — re-key it to the actual parameter or move it to the class-level doc.

⚪ Optional — The toRange doc comment still describes column handling ("Anchors at the resolved line/column … LSP positions are 0-based; sourceLocation is 1-based") and a hasLineCol variable, but this change removed column entirely: toRange(loc: { line: number }) always sets character: 0. The comment now describes behavior the function no longer has, which will mislead the next reader into thinking a column is honored.

Suggested change: Drop the "column" language from the comment and state that the anchor is always at column 0 by design, reflecting the deliberate column drop.

🟡 Recommended — The anyAbortSignal polyfill registers signal.addEventListener('abort', …) on each input signal but never removes the listeners. It is called by retryOnLock on every synth and every assembly refresh, always against the single process-lifetime shutdown signal, so over an active editing session (save → synth, cdk.out change → refresh) these listeners accumulate without bound on one AbortSignal, eventually tripping Node's MaxListenersExceededWarning and holding references for the life of the server. The fresh one-hour AbortSignal.timeout created per retryOnLock call likewise accumulates. This polyfill branch is the real runtime path on Node 18.0–18.16, since AbortSignal.any only landed in 18.17.0 while the repo supports Node >= 18.0.0.

Suggested change: Register the listeners with { once: true } and, when the combined controller aborts, remove any still-attached listeners (track them and clean up in a finally/abort handler); consider creating the one-hour deadline once per server rather than per retryOnLock call.

⚪ Optional — This change only extracts the literal 'synth.lock' into a module-private const SYNTH_LOCK_FILE used solely by the existing constructor. It is not exported and nothing in cdk-explorer imports it, so within this PR it is a pure no-op rename in a different package from the one the change is scoped to. It is harmless, but it is an unrelated hunk (AGENTS.md#pr-conventions, "one concern per PR"). Worth confirming it is intended groundwork for a later change in the stack rather than a stray edit; if it has no consumer here, consider dropping it or splitting it out.

Suggested change: Confirm this constant extraction is intended groundwork for a later change in the stack; if it has no consumer in this PR, drop it or split it into its own change.

🔴 Blocking — The refactor replaces fromCdkApp(app, { env: { JSII_HOST_STACK_TRACES: '1' } }) with fromCdkAppDirectory(projectDir, { synthOptions: { debug: true } }), relying on debug: true to make the synthesized app emit construct creation-stack traces. Those traces are the sole input the assembly index parses into source locations — the basis of every code lens, hover, go-to-definition, and policy diagnostic this package produces. In toolkit-lib, synthParametersFromSettings gates both CDK_DEBUG and JSII_HOST_STACK_TRACES behind the debugApp setting, never debug; the debug→debugApp expansion lives only in the CLI package. So synthOptions: { debug: true } appears to set neither env var, leaving creationStack empty for every node and the Explorer with nothing to anchor. The accompanying unit test was changed to assert nothing about the environment and runs against a fake toolkit, so it cannot catch this.

Suggested change: Verify against a real Toolkit (not the fake) that debug: true actually yields creation stacks; if it does not, pass the setting synthParametersFromSettings reads (debugApp) or set CDK_DEBUG/JSII_HOST_STACK_TRACES through a path toolkit-lib honors, and restore a test asserting the env the app process receives.

🟡 Recommended — This hunk drops the explicit JSII_HOST_STACK_TRACES: '1' env var (commented "Make jsii forward host-language (Python/Java) source frames"), and the companion host-language branch in source-resolver.ts is removed too. Even if TypeScript source mapping is restored, JSII_HOST_STACK_TRACES is only set by toolkit-lib's synthParametersFromSettings under debugApp, so a Python/Java user who previously got go-to-source silently loses it. The PR is titled as a feature-preserving refactor, yet this is an observable behavior change with no stated justification.

Suggested change: Confirm whether host-language support is intentionally dropped. If so, state it in the PR body; if not, keep forwarding host frames so Python/Java source anchoring continues to work.

🟡 Recommended — readAssemblyDirectory calls toolkit.fromAssemblyDirectory(assemblyDir) with no options, but the code this replaces passed { failOnMissingContext: false }. In toolkit-lib AssemblyDirectoryProps.failOnMissingContext defaults to true, so produce() now throws when the assembly's manifest.missing is non-empty — a common state for any cdk.out synthesized with unresolved context lookups. The server catches that rejection into its error branch, which only logs and returns with no cached index, so the Explorer silently shows no constructs, lenses, or diagnostics for that project. The PR body does not mention this behavior change.

Suggested change: Pass { failOnMissingContext: false } to toolkit.fromAssemblyDirectory(...) in readAssemblyDirectory, restoring the prior intent — or, if failing on missing context is deliberate for the Explorer, say so in the PR body.

⚪ Optional — The comment reads // Parsed source maps keyed by absolute .js path; null = known to have no map., but the cache is written by loadTraceMap(jsFile: RelativePath) and read by reverseMapJs via this.cache.get(loc.file) — both using the assembly-relative path, not an absolute one — and the "no map" sentinel is undefined, not null. The comment misleads the next reader about both the key space and the sentinel value.

Suggested change: Update the comment to: "Parsed source maps keyed by assembly-relative .js path; undefined = known to have no map."

⚪ Optional — The first test is titled "reads the app from cdk.json, sets the right environment variables, returns success, and disposes the cached assembly", but after the rewrite it no longer asserts any environment variable — the old JSII_HOST_STACK_TRACES assertion was removed and nothing replaced it, and the fake fromCdkAppDirectory does not inspect synthOptions. The title now describes a check the test does not perform, hiding the untested env-var regression behind an inaccurate name.

Suggested change: Either assert the env/synthOptions the runner now passes, or drop the "sets the right environment variables" clause from the title so it matches what the test checks.

🟡 Recommended — When a synth is queued during a running synth, its resolver is pushed onto synthListeners and the loop drains and resolves listeners each iteration. But the loop's continuation guard is while (synthLatch === 'queued' && !shutdown.signal.aborted). If shutdown.abort() fires while a synth is queued but before the loop takes its next iteration, the loop exits without resolving the already-queued listener, so that queueSynth() promise never resolves. The manual COMMAND_SYNTH_NOW path awaits this promise through notify.withProgress(...), so on shutdown-during-queued-synth the progress indicator can hang unresolved.

Suggested change: In the finally that sets synthLatch = 'open', also drain and resolve any remaining synthListeners so queued callers are released on abort.